Align repository with DPC conventions#62
Conversation
https://github.com/Dans-Plugins/dpc-conventions/blob/main/docs/README_STRUCTURE.md https://github.com/Dans-Plugins/dpc-conventions/blob/main/docs/CONTRIBUTING_STANDARDS.md https://github.com/Dans-Plugins/dpc-conventions/blob/main/docs/DOCUMENTATION_PRACTICES.md https://github.com/Dans-Plugins/dpc-conventions/blob/main/docs/TESTING_AND_CI.md https://github.com/Dans-Plugins/dpc-conventions/blob/main/docs/RELEASE_AUTOMATION.md https://github.com/Dans-Plugins/dpc-conventions/blob/main/docs/GITHUB_COPILOT_INSTRUCTIONS.md Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com> Agent-Logs-Url: https://github.com/Dans-Plugins/Dans-Spawn-System/sessions/f72e4fcf-6ecb-4467-95a5-badac75c0000
There was a problem hiding this comment.
Pull request overview
Aligns the repository with DPC conventions by adding the expected documentation set plus basic CI/release automation and Copilot contributor context.
Changes:
- Rewrites/creates core documentation files (README, guides, commands/config reference, changelog, contributing guide).
- Adds GitHub Actions workflows for build-on-push/PR and release artifact attachment.
- Adds
.github/copilot-instructions.mdto document stack, structure, and contribution workflow for Copilot-assisted changes.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
README.md |
Reworked to match DPC README section structure and link to new docs. |
USER_GUIDE.md |
Adds end-user/admin usage instructions and permissions table. |
COMMANDS.md |
Documents /resetspawn usage and permissions. |
CONFIG.md |
Documents that there’s no config.yml and lists permission nodes. |
CHANGELOG.md |
Introduces Keep a Changelog–style changelog scaffold. |
CONTRIBUTING.md |
Adds contribution workflow and local build/test instructions. |
.github/workflows/build.yml |
CI build workflow on pushes/PRs to main and develop. |
.github/workflows/release.yml |
Release workflow that builds and uploads jars to GitHub Releases. |
.github/copilot-instructions.md |
Captures project stack/structure/conventions for Copilot usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Upload JAR to release | ||
| uses: softprops/action-gh-release@v2 | ||
| with: | ||
| files: target/*.jar |
There was a problem hiding this comment.
files: target/*.jar will likely upload multiple jars when using the Maven Shade plugin (e.g., both the shaded jar and original-*.jar). Consider narrowing the glob to only the intended distributable (or explicitly excluding original-*) so releases don’t contain confusing/incorrect artifacts.
| files: target/*.jar | |
| files: | | |
| target/*.jar | |
| !target/original-*.jar |
| ### Test Server with Plugin Hot-Reloading | ||
|
|
||
| A Docker-based test server can be used for development. | ||
|
|
||
| #### Setup | ||
|
|
||
| 1. Build the plugin: `mvn clean package` | ||
| 2. Copy the resulting jar from `target/` into your server's `plugins/` folder. | ||
| 3. Start your test server. |
There was a problem hiding this comment.
This section mentions a “Docker-based test server”, but there are no Dockerfiles / docker-compose configs in the repo. Either add the referenced Docker setup (and link to it) or remove/adjust this wording so contributors aren’t sent looking for non-existent tooling.
| - name: Set up JDK 17 | ||
| uses: actions/setup-java@v4 | ||
| with: | ||
| java-version: '17' |
There was a problem hiding this comment.
Same concern as build workflow: the repo targets Java 8 bytecode (per pom.xml), but this release workflow builds on JDK 17. Consider switching to JDK 8 or enforcing --release 8 via Maven configuration/toolchains to avoid publishing jars that won’t run on older servers.
| - name: Set up JDK 17 | |
| uses: actions/setup-java@v4 | |
| with: | |
| java-version: '17' | |
| - name: Set up JDK 8 | |
| uses: actions/setup-java@v4 | |
| with: | |
| java-version: '8' |
| - Language: Java | ||
| - Build tool: Maven | ||
| - Target platform: Spigot / Paper (Minecraft plugin, API version 1.13+) | ||
| - Test framework: JUnit (via Maven Surefire) |
There was a problem hiding this comment.
This lists “Test framework: JUnit (via Maven Surefire)”, but pom.xml currently has no JUnit (or other test) dependency configured. Update this to match the actual setup (or add the missing test dependencies/config).
| - Test framework: JUnit (via Maven Surefire) | |
| - Test framework: Not yet configured (add JUnit via Maven Surefire in pom.xml when introducing tests) |
| - `data/` – Persistent data model | ||
| - `bstats/` – bStats metrics integration | ||
| - `src/main/resources/` – `plugin.yml` and other resources | ||
| - `src/test/java/` – Unit tests |
There was a problem hiding this comment.
This references src/test/java/ for unit tests, but the repository currently has no src/test directory. Either add the missing test directory/tests or adjust this project-structure note so it stays accurate.
| - `src/test/java/` – Unit tests | |
| - `src/test/java/` – Unit tests (if/when a test suite is added) |
| ### Unit Tests | ||
|
|
||
| Linux: | ||
|
|
||
| mvn clean test | ||
|
|
||
| Windows: | ||
|
|
||
| mvn.cmd clean test | ||
|
|
||
| If you see `BUILD SUCCESS`, the tests have passed. | ||
|
|
There was a problem hiding this comment.
The README describes running “Unit Tests”, but the repository currently has no src/test/java directory and pom.xml has no JUnit/test dependencies. This makes the Testing section misleading; consider either adding an initial test setup or updating the section to reflect that there are currently no automated tests (and how to validate changes instead).
| ### Unit Tests | |
| Linux: | |
| mvn clean test | |
| Windows: | |
| mvn.cmd clean test | |
| If you see `BUILD SUCCESS`, the tests have passed. | |
| ### Automated Tests | |
| This project does not currently include automated unit tests. | |
| To verify that the project builds successfully, run: | |
| Linux: | |
| mvn clean package | |
| Windows: | |
| mvn.cmd clean package | |
| If you see `BUILD SUCCESS`, the project compiled successfully. | |
| For validating functional changes, follow the steps in the **Development** section below to run the plugin on a test server. |
| 2. Copy the resulting jar from `target/` into your server's `plugins/` folder. | ||
| 3. Start your test server. | ||
|
|
||
| ## Authors and Acknowledgement |
There was a problem hiding this comment.
PR description says the README follows the required section names/order (including an “Authors” section). The README uses “Authors and Acknowledgement” instead, which may not satisfy tooling or checklist expectations that look for the exact “Authors” heading. Consider renaming this heading to match the convention/PR claim.
| | `spawnsystem.reset.others` | op | Allows resetting another player's spawn | | ||
| | `spawnsystem.admin` | op | Grants all plugin permissions | | ||
|
|
||
| These defaults are defined in `src/main/resources/plugin.yml` and can be overridden by your permissions plugin. |
There was a problem hiding this comment.
This line says the permission defaults are defined in src/main/resources/plugin.yml, but the current plugin.yml does not declare these spawnsystem.* nodes (it declares different nodes). Either update plugin.yml to match the documented permission nodes/defaults, or adjust this documentation so it doesn’t point to an incorrect source of truth.
| These defaults are defined in `src/main/resources/plugin.yml` and can be overridden by your permissions plugin. | |
| By default, these permissions are granted to server operators (`op`) and can be overridden by your permissions plugin. |
| Run the unit tests with: | ||
|
|
||
| Linux: `mvn clean test` | ||
| Windows: `mvn.cmd clean test` | ||
|
|
There was a problem hiding this comment.
This section instructs contributors to run “unit tests”, but there is currently no src/test/java directory and pom.xml has no test dependencies configured. Consider updating this section to state that there are no automated tests yet (and what manual verification is expected), or add a minimal test setup so these instructions are actionable.
| Run the unit tests with: | |
| Linux: `mvn clean test` | |
| Windows: `mvn.cmd clean test` | |
| There are currently no automated unit tests configured for this project. | |
| Please verify your changes manually by: | |
| - Building the plugin: `mvn clean package` | |
| - Copying the built JAR from `target/` into your test server's `plugins` folder | |
| - Starting the server and ensuring the plugin loads without errors in the console | |
| - Exercising relevant spawn-related functionality to confirm your changes behave as expected |
| - name: Set up JDK 17 | ||
| uses: actions/setup-java@v4 | ||
| with: | ||
| java-version: '17' |
There was a problem hiding this comment.
The project is configured to compile for Java 8 (pom.xml sets <java.version>1.8</java.version>), but this workflow builds on JDK 17. Building on 17 without using --release 8/toolchains can accidentally allow compilation against newer JDK APIs and produce artifacts that won’t run on Java 8 servers. Consider building on JDK 8 (and optionally also 17), or update Maven compiler config to use maven.compiler.release=8 (or toolchains) to enforce Java 8 compatibility.
| - name: Set up JDK 17 | |
| uses: actions/setup-java@v4 | |
| with: | |
| java-version: '17' | |
| - name: Set up JDK 8 | |
| uses: actions/setup-java@v4 | |
| with: | |
| java-version: '8' |
|
@copilot address comments |
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com> Agent-Logs-Url: https://github.com/Dans-Plugins/Dans-Spawn-System/sessions/153529f8-d2f8-4945-b033-b7ab7908dc10
All reviewer comments addressed in commit
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | Permission Node | Default | Description | | ||
| |-----------------|---------|-------------| | ||
| | `spawnsystem.placeSpawnSign` | op | Allows placing `[Spawn]` signs | | ||
| | `spawnsystem.breakSpawnSign` | op | Allows breaking `[Spawn]` signs | | ||
| | `spawnsystem.reset.self` | op | Allows a player to reset their own spawn | | ||
| | `spawnsystem.reset.others` | op | Allows resetting another player's spawn | | ||
| | `spawnsystem.admin` | op | Grants all plugin permissions | |
There was a problem hiding this comment.
The permissions table is using || at the start of each row, which renders as an extra empty column in Markdown. Switch to standard Markdown table formatting (single leading |, consistent trailing |).
| **Description:** Resets a player's custom spawn point, returning them to the world's default spawn on next death. | ||
| **Permission (no argument):** `spawnsystem.reset.self` or `spawnsystem.admin` | ||
| **Permission (with argument):** `spawnsystem.reset.others` or `spawnsystem.admin` | ||
| **Usage:** `/resetspawn` or `/resetspawn <player>` |
There was a problem hiding this comment.
The command currently only performs any action when executed by an in-game player (the implementation returns early for non-player senders). The docs should mention that /resetspawn is player-only, or the command should be updated to support console execution (with appropriate feedback).
| **Usage:** `/resetspawn` or `/resetspawn <player>` | |
| **Usage:** (in-game players only) `/resetspawn` or `/resetspawn <player>` |
| | Name | Main Contributions | | ||
| |------|--------------------| | ||
| | DanTheTechMan | Original author and lead developer | |
There was a problem hiding this comment.
The Developers table is using || at the start of the header/rows, which creates an extra empty column in Markdown. Use a single leading | (and consistent trailing |) so the table renders as intended.
|
@copilot address comments |
… README heading Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com> Agent-Logs-Url: https://github.com/Dans-Plugins/Dans-Spawn-System/sessions/b0b63dde-9578-4420-8558-db37bf9a98b7
All reviewer comments addressed in commit
Regarding the |
permissions: contents: read)original-*.jarfrom release assets)spawnsystem.*notdansplugins.*); add missingspawnsystem.adminnode/resetspawnusage lineOriginal prompt
📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.